Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Curl Callback #276

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Curl Callback #276

wants to merge 5 commits into from

Conversation

whobutsb
Copy link
Contributor

@whobutsb whobutsb commented Nov 11, 2022 •
edited
Loading

I've been working on extending some of the functionality of the phpclassic/php-shopify library as a Laravel Service Provider. One of the things that I wish I could do for every Curl request is to log the curl headers and response in a centralized place, rather than calling \PHPShopify\CurlRequest::$lastHttpResponseHeaders; After every API request that I make.

So I have added a feature that allows a user to set a callback to receive the CurlResponse information after every response.

Usage:

\PHPShopify\CurlRequest::setCurlCallback(function(CurlResponse $response, CurlHandle $curl) {

    // do something with the curl response data

    \Log::info('Shopify API Response', [
            'shopify_response_headers' => $response->getHeaders(),
             'request_time_elapsed' => curl_getinfo($curl, CURLINFO_TOTAL_TIME),
    ]);
});

I hope others will find this functionality useful. Thank you for the consideration.

UPDATE: Also now including the CurlHandle in the arguments. It is helpful to be able to log the curl requests total elapsed time.

@whobutsb whobutsb mentioned this pull request Dec 13, 2022
Merged
@whobutsb
Copy link
Contributor Author

Hi @tareqtms, sorry to bother you. I was wondering if this could be considered as a new feature in the library? I've found it be very helpful in my projects to log API requests, especially when talking with Shopify Support who would like information about the request response headers.

Thank you for the consideration.

@tareqtms
Copy link
Contributor

@whobutsb
I will merge it soon after checking, thanks.

@whobutsb whobutsb mentioned this pull request Jun 27, 2023
Merged
@tareqtms
Copy link
Contributor

@whobutsb
Please check my review comments.

@whobutsb
Merge branch 'master' into feature/curl-callback
1c36685 
# Conflicts:
#	lib/CurlRequest.php
@whobutsb
Copy link
Contributor Author

@tareqtms - I have resolved the merge conflicts. I'm not seeing any of your comments. I believe you need to submit the comments in the code section in the top right hand corner first for me to see what you commented on. I had this issue before working with other developers.

@whobutsb
Copy link
Contributor Author

whobutsb commented Jul 3, 2023

Hi @tareqtms, looking forward to seeing your comments. Could you please submit them so I can review them and fix anything you requested. Thank you very much!

@whobutsb whobutsb mentioned this pull request Aug 30, 2023
Merged
@tareqtms
Copy link
Contributor

Not sure why you can't see the comments. Here is a screenshot.
Screenshot 2023-09-16 at 11 38 18 AM

@whobutsb
Copy link
Contributor Author

whobutsb commented Sep 18, 2023 •
edited
Loading

Hi @tareqtms - Thank you for replying with the image and considering this PR. I have been using this curl callback on my own branch for a while and it has been very helpful to retrieve additional details which can be used by Shopify support to help diagnose issues.

The $ch is referring to the CurlHandle object. I used $ch because it is what you have used in your code previously in your CurlRequest.php class. The CurlHandle object is helpful in logging to retrieve additional details about the curl request. In my apps I'am using it to return the full url and also the elapsed time of the request. Example:

'resource_url' => curl_getinfo($ch, CURLINFO_EFFECTIVE_URL),
'time_elapsed' => curl_getinfo($ch, CURLINFO_TOTAL_TIME),

There are also lot so additional helpful methods you can use to get important information about the curl request. See: https://www.php.net/manual/en/function.curl-getinfo.php

I have updated the Example to read "Shopify API Response" to help avoid confusion.

Also I can see you have written the comments but you have not "requested" them so that I can see them. On the "Files changed" page in the top right hand corner you need to submit them with the "request changes" radio button. See:
Screenshot 2023-09-18 at 7 41 07 AM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@whobutsb @tareqtms